Cratewide API privatisation#318
Merged
wprzytula merged 6 commits intoscylladb:masterfrom Jun 11, 2025
Merged
Conversation
Contributor
Author
|
CI failed because of clippy. Once UDT is no longer part of public API, clippy starts complaining about its name violating the conventions. |
Lorak-mmk
reviewed
Jun 10, 2025
Contributor
Lorak-mmk
left a comment
Lorak-mmk
left a comment
There was a problem hiding this comment.
Why did you decide to not move the commit changing the privacy to the end? This way no commit would emit warnings.
Two functions in `metadata.rs` were missing the `#[no_mangle]` attribute, which is necessary for FFI compatibility: - `cass_materialized_view_meta_partition_key_count` - `cass_materialized_view_meta_clustering_key` This commit adds the attribute to ensure that these functions can be correctly linked from C code.
This serves two purposes: 1. Explicit is better for readability. 2. It avoids a warning about `CASS_COMPRESSION_NONE` being unused once we privatise the module which imports it (in next commits).
UDTDataType had two methods that were not used anywhere in the codebase: - `add_field`; - `get_field_by_index`. These methods were removed not to clutter the codebase with unused code.
The field was not being used anywhere in the codebase, so it's removed.
wprzytula
force-pushed
the
cratewide-privatisation
branch
from
2bb11ad to
ac1afb6
Compare
This is a big commit that privatises a lot of items which have been `pub` for no good reason. The goal is to make the public API of the wrapper clear and minimal, and to prevent silencing warnings about unused items (because `pub` items are always considered used). I've kept the `argconv` module public, together with its traits and types, because it's used in doctests. Privatising it would make them stop compiling. Also, those type and traits are actually part of the API, in a sense that they are FFI-facing. So let them stay public.
wprzytula
force-pushed
the
cratewide-privatisation
branch
from
ac1afb6 to
ea47ecd
Compare
So that checks pass without complaints.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Lots of items in the driver are exposed as
pubfor no good reason. This involves:pubstructs and enums,pubtraits,pubfields,pubfunctions and methods.This has a number of drawbacks:
cassandra.h.pubitems are considered used by the compiler.pubfunctions must be exposed as proper (noninlined) functions, and they must obey calling conventions.pubtypes and theirpubfields must not be individually optimised, so that other crates can understand how to use them.So privatisation is profitable!
What's done
Privatisation
I perused the whole crate and replaced
pubwithpub(crate)(or evenpub(self)) where possible.Found & removed a bug in materialized views API
Two functions in
metadata.rswere missing the#[no_mangle]attribute, which is necessary for FFI compatibility:cass_materialized_view_meta_partition_key_count;cass_materialized_view_meta_clustering_key.So I added the missing attribute. I have an idea how to prevent similar bugs - will open an issue.
Cleanup after privatisation
A number of items were found to be unused. I either removed them or silenced the warnings.
Pre-review checklist
[ ] I have enabled appropriate tests in.github/workflows/build.ymlingtest_filter.[ ] I have enabled appropriate tests in.github/workflows/cassandra.ymlingtest_filter.